feat: support Android apps in worker heartbeat and UI#14
Conversation
- Accept top-level `apps` array in heartbeat (alongside `containers`) - Store apps in new `apps` column on workers table (auto-migrated) - Convert Android app statuses to service entries for the dashboard - Fleet summary counts apps for Android workers - Fleet UI shows "Android" badge and renders apps distinctly - Dashboard disables Docker actions for Android instances
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 11 minutes and 28 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds Android worker support: database gains an Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/main.py (1)
1562-1571: Consider extracting duplicate JSON parsing logic.The fleet summary duplicates the JSON parsing and count logic that
_parse_worker_json()already handles. Consider reusing it:Suggested refactor
for w in workers: - sys_info = json.loads(w.get("system_info", "{}")) - is_android = sys_info.get("device_type") == "android" - if is_android: - apps = json.loads(w.get("apps", "[]")) - total_containers += len(apps) - total_running += sum(1 for a in apps if a.get("running")) - else: - containers = json.loads(w.get("containers", "[]")) - total_containers += len(containers) - total_running += sum(1 for c in containers if c.get("status") == "running") + _parse_worker_json(w) + total_containers += w["container_count"] + total_running += w["running_count"] if w["status"] == "online": online_workers += 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/main.py` around lines 1562 - 1571, The code duplicates JSON parsing and counting for workers; instead call the existing helper _parse_worker_json(w) (or adapt it to return both a list of containers/apps and the running count) and use its outputs to increment total_containers and total_running; replace the sys_info/json.loads blocks and the is_android branch with a single call to _parse_worker_json(w) and then do total_containers += len(containers) and total_running += running_count (or derive running_count from the returned containers), ensuring _parse_worker_json handles both "apps" and "containers" and returns a consistent structure.app/database.py (1)
181-185: Consider logging when migration adds the new column.The migration silently adds the
appscolumn. Adding a log statement would help operators understand schema changes during upgrades.Suggested improvement
if "apps" not in cols: await db.execute("ALTER TABLE workers ADD COLUMN apps TEXT NOT NULL DEFAULT '[]'") + _logger.info("Migrated workers table: added 'apps' column")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/database.py` around lines 181 - 185, The migration silently adds the apps column; update the block that checks PRAGMA table_info(workers) and executes ALTER TABLE workers ADD COLUMN apps ... to emit a log entry when the column is added. Use the project's existing logger (e.g., logger or process_logger) and log at info/debug level with a clear message like "Added 'apps' column to workers table during migration" and include the default value and SQL executed; keep the log right after the await db.execute(...) that adds the column so operators can see schema changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/database.py`:
- Around line 181-185: The migration silently adds the apps column; update the
block that checks PRAGMA table_info(workers) and executes ALTER TABLE workers
ADD COLUMN apps ... to emit a log entry when the column is added. Use the
project's existing logger (e.g., logger or process_logger) and log at info/debug
level with a clear message like "Added 'apps' column to workers table during
migration" and include the default value and SQL executed; keep the log right
after the await db.execute(...) that adds the column so operators can see schema
changes.
In `@app/main.py`:
- Around line 1562-1571: The code duplicates JSON parsing and counting for
workers; instead call the existing helper _parse_worker_json(w) (or adapt it to
return both a list of containers/apps and the running count) and use its outputs
to increment total_containers and total_running; replace the sys_info/json.loads
blocks and the is_android branch with a single call to _parse_worker_json(w) and
then do total_containers += len(containers) and total_running += running_count
(or derive running_count from the returned containers), ensuring
_parse_worker_json handles both "apps" and "containers" and returns a consistent
structure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 33aa87db-04d6-478f-b0cf-6fed8bffb861
⛔ Files ignored due to path filters (3)
app/static/favicon.svgis excluded by!**/*.svgdocs/icon.svgis excluded by!**/*.svgdocs/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (4)
app/database.pyapp/main.pyapp/static/js/app.jsapp/templates/fleet.html
- Resilient JSON parsing: _safe_json() helper so one malformed DB row degrades that worker instead of 500-ing the whole fleet/services UI - Fleet summary now filters offline workers (was counting stale data) - Fleet UI: "Containers" label → "Services", onboarding text mentions Android app, app pills show 24h net traffic on hover tooltip - Dashboard sub-rows show net_tx/rx for Android instances instead of CPU/memory placeholders
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/templates/fleet.html (1)
142-147: Inconsistency withfmtNetBytesin app.js — missing null guard.Line 143 returns
b + ' B'which would produce"undefined B"or"null B"if called with falsy non-zero values. The equivalent function inapp/static/js/app.js:77uses(b || 0) + ' B'.Currently safe because lines 156-157 pre-default values with
|| 0, but the function isn't robust standalone. Also consider extracting this to a shared utility to avoid duplication.Proposed fix
function fmtBytes(b) { - if (!b || b < 1024) return b + ' B'; + if (!b || b < 1024) return (b || 0) + ' B'; if (b < 1048576) return (b / 1024).toFixed(1) + ' KB'; if (b < 1073741824) return (b / 1048576).toFixed(1) + ' MB'; return (b / 1073741824).toFixed(2) + ' GB'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/templates/fleet.html` around lines 142 - 147, The fmtBytes function can produce "undefined B" or "null B" for falsy non-zero inputs; update fmtBytes to defensively default b to 0 (e.g., treat b as (b || 0)) at the start of the function so it mirrors fmtNetBytes behavior and never returns "undefined B", and then consider extracting fmtBytes (and fmtNetBytes) into a shared utility to remove duplication and keep formatting logic consistent across app/templates/fleet.html and app/static/js/app.js.app/static/js/app.js (1)
523-528: Consider adding a visual indicator that CPU/Memory columns show network metrics for Android.The table headers remain "CPU" and "Memory" (lines 342-343), but Android sub-rows display network TX/RX. Users may be confused seeing byte values in these columns. A small inline label or tooltip on the cells could clarify this.
Otherwise the logic is sound — Android instances correctly show network metrics while non-Android instances retain CPU/memory display.
Also applies to: 532-532, 538-539
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/static/js/app.js` around lines 523 - 528, The CPU/Memory columns show network metrics for Android instances (inst.is_android) but lack any visual cue; update the code that builds cpuCell and memCell so when inst.is_android you append a short inline indicator or tooltip (e.g., " (net TX)" / " (net RX)" or a span with title attribute) to the displayed fmtNetBytes value, and ensure the same change is applied where cpuCell/memCell are used (refer to variables cpuCell, memCell and the inst.is_android branch); keep the existing formatting for non-Android instances.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/static/js/app.js`:
- Around line 523-528: The CPU/Memory columns show network metrics for Android
instances (inst.is_android) but lack any visual cue; update the code that builds
cpuCell and memCell so when inst.is_android you append a short inline indicator
or tooltip (e.g., " (net TX)" / " (net RX)" or a span with title attribute) to
the displayed fmtNetBytes value, and ensure the same change is applied where
cpuCell/memCell are used (refer to variables cpuCell, memCell and the
inst.is_android branch); keep the existing formatting for non-Android instances.
In `@app/templates/fleet.html`:
- Around line 142-147: The fmtBytes function can produce "undefined B" or "null
B" for falsy non-zero inputs; update fmtBytes to defensively default b to 0
(e.g., treat b as (b || 0)) at the start of the function so it mirrors
fmtNetBytes behavior and never returns "undefined B", and then consider
extracting fmtBytes (and fmtNetBytes) into a shared utility to remove
duplication and keep formatting logic consistent across app/templates/fleet.html
and app/static/js/app.js.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: de118c8a-2260-4e6f-a36a-a641bda51266
📒 Files selected for processing (3)
app/main.pyapp/static/js/app.jsapp/templates/fleet.html
🚧 Files skipped from review as they are similar to previous changes (1)
- app/main.py
Workers are now identified by client_id (UNIQUE) instead of name. Existing workers are migrated with client_id = name for backward compat. The heartbeat accepts an optional client_id field; when absent, name is used as fallback. This prevents two devices with the same display name from overwriting each other's state. Also adds 8 tests covering heartbeat identity, fleet summary (online-only filtering, Android app counting, malformed JSON resilience), and worker list (Android vs Docker counting).
- Migration now recreates the idx_workers_status index after table rebuild (was lost on upgraded installs) - Log migration with _logger.info for observability - Fleet summary reuses _parse_worker_json() instead of inline parsing - fmtBytes() null-safe for undefined/zero values
Show ↑/↓ prefixes so it's clear the CPU/Memory columns display network traffic for Android instances.
Summary
appsarray in worker heartbeat payload (alongsidecontainers), matching the schema change in CashPilot-android PR Test: verify database schema creation and earnings CRUD #6appscolumn onworkerstable (auto-migrated viaALTER TABLEon startup)system_info.device_type == "android") have their apps converted to service entries for the main dashboard — they appear like any other deployed service with running/stopped statusCompanion PR
CashPilot-android #6 — moves apps from being faked as
containersto a proper top-levelappsfieldTest plan
appsarray stores and displays correctlyappscolumn on existing databases without data lossSummary by CodeRabbit
New Features
UI / Content